cuda.core: add graph definition node updates#2395
Draft
Andy-Jost wants to merge 6 commits into
Draft
Conversation
Andy-Jost
added
P0
Contributor
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
Contributor
Author
|
/ok to test |
|
Use the generic node setter with failure-atomic attachment replacement, establishing the shared path for definition-level parameter mutation.
Extend definition-level mutation to event waits and both Python and ctypes host callbacks while preserving old executable state and attachment ownership.
Report unsupported driver or binding versions before preparing mutation attachments or calling the generic node setter.
Andy-Jost
force-pushed
the
graph-definition-node-updates
branch
from
July 22, 2026 23:04
4ce2862 to
fffa9fd
Compare
Allow partial memset parameter replacement while preserving graph-owned destination lifetimes and previously instantiated graph behavior.
Support partial copy parameter replacement while preserving independent source and destination ownership across graph instantiations.
mdboom
reviewed
Jul 23, 2026
mdboom
left a comment
mdboom
left a comment
Contributor
There was a problem hiding this comment.
The general organization and approach looks fine to me.
One small suggestion (that I can't really evaluate because I don't deeply understand the use cases).
Comment on lines
+388
to
+397
| def update( | ||
| self, | ||
| dst: Buffer | int | None = None, | ||
| value=None, | ||
| width: int | None = None, | ||
| height: int | None = None, | ||
| pitch: int | None = None, | ||
| *, | ||
| dst_owner=None, | ||
| ) -> None: |
Contributor
There was a problem hiding this comment.
One minor point (which really hinges on how we expect these update methods to be used).
If the idea is that users will most often be updating a subset of these values, it might be clearer to make them all kwarg only (i.e. put the *, at the beginning).
It's a minor point, but something we won't be able to change after the API freezes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Add atomic parameter mutation for graph definition nodes, starting with event-record nodes. This draft establishes the shared generic-setter and attachment transaction path that the remaining node types will use.
This branch is stacked on #2357 for early CI coverage and will be rebased onto
mainafter that PR merges.Changes
cuGraphNodeSetParams.EventRecordNode.update()with failure-atomic attachment replacement and cached-state commit.Test Coverage
ipp1-1093.pre-commit run --all-files.Related Work